-
-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix the recording of arcs for non-Python files #1051
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1051 +/- ##
==========================================
+ Coverage 93.97% 94.09% +0.11%
==========================================
Files 179 86 -93
Lines 25140 12235 -12905
Branches 2613 1230 -1383
==========================================
- Hits 23626 11512 -12114
+ Misses 1210 582 -628
+ Partials 304 141 -163 Continue to review full report at Codecov.
|
Thanks, I don't often get contributions to the C code! Sorry it's taken me so long to respond. Do you have a way to reproduce the problem this is fixing? BTW, separately, I'd be interested to hear about the plugin you are working on. |
I don't have a simple way of reproducing the problem. I discovered it when running liberapay's test suite with the experimental coverage plugins, and I confirmed that the changes in this branch fix the problem, but I didn't write test code that reproduces it and could be used to prevent regressions in the future. The plugins I worked on are for Jinja2 and simplates. I obtained some encouraging results, but the work isn't finished, I paused after I opened this pull request. |
c4f13e0
to
96e19c3
Compare
I've rebased this branch on master, replacing |
0c14f1a
to
82169a6
Compare
since this is so old, updates have happened since, and 3 checks are not being met ... is this something that can be closed? |
This patch modifies the C code so that it no longer mixes mapped and unmapped line numbers. In other words, the line numbers are now always passed through the tracer's `line_number_range` method before being recorded. This patch also modifies the C code to clear the `DataStackEntry` structs before reusing them. Not clearing their `last_line` attribute can result in bogus arcs being recorded.
The "missing-return handler" was dropped in 8021196.
96e19c3
to
b712f22
Compare
This patch is still needed. I have now rebased it on master once again, and confirmed that it still fixes the problem. The patch has become a bit smaller because its previous version modified the The patch has also been modified to use |
Can you give me very specific instructions for cloning and running the test suite, and specific pointers to places where the coverage information is wrong? I took a quick look in the liberapay repo and couldn't find a coverage plugin. |
Hi,
While developing new coverage plugins I found that they received invalid arcs when branch coverage was turned on.
An example of an obviously invalid arc is one which contains a line number greater than the actual number of lines in the file. This can happen when the generated python code being executed is longer than the source code it was generated from.
This problem has two causes:
line_number_range
method before being recorded.DataStackEntry
struct isn't cleared before being reused. Not clearing itslast_line
attribute results in bogus arcs being recorded.My patch fixes both problems.
I don't often write C code, so please let me know if you see something that I could have done better.